Skip to content

feat: replace yarn with pnpm#471

Merged
tjgofore merged 1 commit into
mainfrom
feat/PT-2045-replace-yarn-with-pnpm
Jun 8, 2026
Merged

feat: replace yarn with pnpm#471
tjgofore merged 1 commit into
mainfrom
feat/PT-2045-replace-yarn-with-pnpm

Conversation

@tjgofore

@tjgofore tjgofore commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Description ✨

  • update node to version 24
  • replace yarn with pnpm
  • add update-runtime-env script for creating env config file

Issues 🐛

Closes 🙅‍♀️

PT-2045

Refs: PT-2045

Related 🤝

Testing ⚗️

Automated tests ⚙️️

Manual testing 👷‍♂️

Screenshots 📸

Additional notes 🗒️

Summary by CodeRabbit

  • New Features

    • Runtime environment injection for frontend with a runtime env template; centralized runtime env access.
    • Dedicated test-mode environment configuration for automated tests.
  • Chores

    • Migrated project from Yarn to pnpm; updated scripts, CI wiring, and tooling.
    • Modernized Docker multi-stage pnpm build and bumped Node requirement to v24.x; default dev port changed to 8080.
  • Documentation

    • Updated README and Playwright/test docs for pnpm workflows.
  • Tests

    • Added Vitest coverage for env utilities and stabilized test mocks.

@tjgofore tjgofore requested a review from a team as a code owner June 3, 2026 04:32
@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch 5 times, most recently from a456e8f to 3247e46 Compare June 3, 2026 08:58
@azure-pipelines

Copy link
Copy Markdown

KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀

@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 3247e46 to dd6b2b2 Compare June 3, 2026 10:39
Comment thread Dockerfile
@azure-pipelines

Copy link
Copy Markdown

KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀

@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch 2 times, most recently from 145de61 to 15c2363 Compare June 3, 2026 11:04
@azure-pipelines

Copy link
Copy Markdown

KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀

@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 15c2363 to 5be2023 Compare June 3, 2026 11:35
Comment thread scripts/update-runtime-env.ts Outdated
Comment thread playwright.config.ts Outdated
@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 5be2023 to 2e3717f Compare June 3, 2026 11:43
Comment thread src/domain/app/AppConfig.ts Outdated
@azure-pipelines

Copy link
Copy Markdown

KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀

@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 2e3717f to 0c211d9 Compare June 3, 2026 11:59
Comment thread Dockerfile
@azure-pipelines

Copy link
Copy Markdown

KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀

@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch 2 times, most recently from dc89771 to 07e6a30 Compare June 4, 2026 03:29
@azure-pipelines

Copy link
Copy Markdown

KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀

@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 07e6a30 to 8d7c7d0 Compare June 4, 2026 03:44
@azure-pipelines

Copy link
Copy Markdown

KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀

@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 8d7c7d0 to 46b8538 Compare June 4, 2026 04:05
Comment thread src/domain/app/AppConfig.ts
@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 46b8538 to 1fde8eb Compare June 4, 2026 04:11
@azure-pipelines

Copy link
Copy Markdown

KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/test/apollo-mocks/headerMenuMock.ts (1)

15-527: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update remaining padded base64 menu IDs in other mocks
The repo still contains legacy padded base64 IDs in src/test/apollo-mocks/footerMenuMock.ts (e.g., menu.id: 'dGVybTo1Nw==' and menuItems.nodes[].id: 'cG9zdDo0MjY=', cG9zdDo0Mjc='). If the change to unpadded IDs was meant to be consistent, update this file as well; otherwise, document that padded IDs are intentionally kept there.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/test/apollo-mocks/headerMenuMock.ts` around lines 15 - 527, The footer
mock still uses padded base64 IDs (e.g., menu.id 'dGVybTo1Nw==' and
menuItems.nodes[].id 'cG9zdDo0MjY=', 'cG9zdDo0Mjc=') while headerMenuMock uses
unpadded IDs; update src/test/apollo-mocks/footerMenuMock.ts to use the unpadded
forms (remove trailing '=' padding) for the same fields so they match the
MenuQueryResponse shape used elsewhere (update the values for menu.id and each
menuItems.nodes[].id), or add a short comment in the file documenting that
padded IDs are intentionally kept if that was the deliberate choice.
🧹 Nitpick comments (5)
scripts/update-runtime-env.ts (1)

28-31: ⚡ Quick win

Use path.join for cross-platform compatibility.

Line 30 uses string concatenation to build the path, which is less robust than path.join.

♻️ Proposed fix
 const configurationFile: string = path.join(
   __dirname,
-  '../public/' + configFile
+  '../public',
+  configFile
 );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/update-runtime-env.ts` around lines 28 - 31, The path for
configurationFile is built with string concatenation; update the construction to
use path.join for cross-platform correctness. In the configurationFile
assignment (the variable named configurationFile that currently uses __dirname
and configFile), call path.join with components (e.g., __dirname, '..',
'public', configFile) instead of concatenating '../public/' + configFile so
separators are handled correctly across OSes.
.github/workflows/ci.yml (1)

11-16: ⚡ Quick win

Consider adding explicit permissions to reduce attack surface.

The workflow inherits default permissions which may be overly broad. Explicitly defining minimal permissions follows the principle of least privilege.

🛡️ Proposed explicit permissions
 jobs:
   common:
+    permissions:
+      contents: read
+      checks: write
+      pull-requests: write
     uses: City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@main

Note: Adjust the permissions based on what the reusable workflow actually requires.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 11 - 16, The workflow currently
inherits default permissions for the reusable job "common" (uses:
City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@main); add an
explicit permissions block under that job to restrict token scopes to the
minimum required by the reusable workflow (e.g., explicitly set permissions:
contents: read, packages: read, pull-requests: write or whatever the reusable
workflow needs) instead of relying on defaults—review the reusable workflow's
steps to determine exact scopes and add only those permission keys.
src/utils/__tests__/envUtils.test.ts (1)

5-34: ⚡ Quick win

Consider adding a test for the process.env fallback path.

The tests validate resolution from window._env_ and import.meta.env, but the middle fallback—process.env—is untested. According to the implementation, this path is used by Vitest and server-side tooling when runtime injection is unavailable.

🧪 Proposed test to cover process.env fallback
   it('falls back to import.meta.env when runtime and process env are missing', () => {
     (globalThis.window as Window & { _env_?: Record<string, unknown> })._env_ =
       undefined;
     delete process.env.BASE_URL;
 
     expect(getEnvValue('BASE_URL')).toBe(import.meta.env.BASE_URL);
   });
+
+  it('falls back to process.env when runtime env is missing', () => {
+    (globalThis.window as Window & { _env_?: Record<string, unknown> })._env_ =
+      undefined;
+    process.env.TEST_ENV_UTILS_VALUE = 'process-env-value';
+
+    expect(getEnvValue('TEST_ENV_UTILS_VALUE')).toBe('process-env-value');
+  });
 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/__tests__/envUtils.test.ts` around lines 5 - 34, Add a test that
verifies getEnvValue falls back to process.env: set (globalThis.window as Window
& { _env_?: Record<string, unknown> })._env_ = undefined, assign
process.env.TEST_ENV_UTILS_VALUE = '456' (or another distinct value), then
assert expect(getEnvValue('TEST_ENV_UTILS_VALUE')).toBe('456'); name the test to
reflect the "process.env fallback" and rely on the existing afterEach cleanup
that deletes TEST_ENV_UTILS_VALUE and restores process.env/runtime env.
.env.test (1)

5-6: ⚡ Quick win

Remove placeholder syntax that doesn't expand.

The $npm_package_* syntax won't be expanded by dotenv and is overridden by the generator script. These lines are effectively ignored since the generator explicitly reads from package.json.

♻️ Suggested fix
 VITE_APP_CMS_URI=https://kultus.app-staging.hkih.hion.dev/graphql
-VITE_APP_APPLICATION_NAME=$npm_package_name
-VITE_APP_VERSION=$npm_package_version
+# VITE_APP_APPLICATION_NAME and VITE_APP_VERSION are auto-populated from package.json
 
 # Sentry
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.env.test around lines 5 - 6, Remove the non-expanding placeholder
assignments for VITE_APP_APPLICATION_NAME and VITE_APP_VERSION in .env.test (the
lines using $npm_package_name and $npm_package_version); either delete these two
lines or replace them with concrete values (e.g., the actual app name and
version) so dotenv isn't given unusable placeholders—note the generator already
pulls these from package.json, so keep .env.test free of $npm_package_*
placeholders and rely on the generator or explicit literal values instead.
.env.runtime.template (1)

23-24: ⚡ Quick win

Document that these variables are auto-populated.

VITE_APP_APPLICATION_NAME and VITE_APP_VERSION are automatically overridden by the generator script from package.json, so any values set here will be ignored. Consider adding a comment to clarify this behavior or removing them from the template entirely.

📝 Suggested documentation fix
 VITE_APP_IDLE_TIMEOUT_IN_MS=
-VITE_APP_APPLICATION_NAME=
-VITE_APP_VERSION=
+# VITE_APP_APPLICATION_NAME=  # Auto-populated from package.json
+# VITE_APP_VERSION=            # Auto-populated from package.json
 VITE_APP_COMMITHASH=
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.env.runtime.template around lines 23 - 24, Add a short comment above
VITE_APP_APPLICATION_NAME and VITE_APP_VERSION explaining these values are
auto-populated/overridden by the generator script from package.json (so values
in this template are ignored) or remove these lines entirely; update the
.env.runtime.template to either include that explanatory comment referencing
VITE_APP_APPLICATION_NAME and VITE_APP_VERSION or delete those two entries to
avoid confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.dockerignore:
- Around line 7-8: Remove the allowlist entries that expose local secret files
(the lines matching "!.env" and "!.env*") and instead explicitly allow only
non-secret template files (for example add patterns like "!.env.example" or
"!.env.template" or specific template filenames you ship) so real .env files
remain excluded from the Docker build context; update the .dockerignore by
deleting the "!.env" / "!.env*" entries and adding explicit template patterns
for any env templates you need to include.

In @.env.test:
- Line 29: The VITE_APP_OIDC_SCOPE environment variable is unquoted and contains
spaces; update the VITE_APP_OIDC_SCOPE entry to wrap the multi-word value in
quotes (e.g., "openid profile email") so dotenv parses it consistently (match
the quoted style used in .env.example) and ensure any tooling that reads
VITE_APP_OIDC_SCOPE receives the full space-separated scope string.

In @.github/workflows/ci.yml:
- Line 12: Replace the mutable branch ref used for the reusable workflow with a
pinned commit SHA: update the uses entry
"City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@main" to point to
the exact commit hash (i.e.
"City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@<commit-sha>")
obtained from the repo (for example via the gh API or git) so the workflow is
immutable and supply-chain secure.

In `@package.json`:
- Line 116: The package.json npm script "test:debug" contains a typo in its
flags: replace the incorrect "----no-file-parallelism" with the correct
"--no-file-parallelism" in the "test:debug" script so the flag uses two hyphens;
update the value of the "test:debug" entry to use the corrected flag.

In `@pipelines/kultus-admin-ui-review.yml`:
- Around line 44-45: Remove the temporary pinned pipeline ref by deleting the
line containing "ref: refs/heads/feat/PT-2045-replace-yarn-with-pnpm" (and the
preceding TODO comment) in the pipelines/kultus-admin-ui-review.yml so the
pipeline no longer targets this PR branch; if a stable branch or tag is required
instead, replace that value with the intended branch/ref.

In `@pnpm-workspace.yaml`:
- Around line 1-3: The current minimumReleaseAge in pnpm-workspace.yaml is set
to 10 minutes which weakens supply-chain protections; update the
minimumReleaseAge setting to a more secure value (e.g., 1440 for 24 hours or
10080 for 1 week) in the pnpm-workspace.yaml file by changing the
minimumReleaseAge entry to the chosen higher integer to allow time for community
vetting and reduce risk from compromised or malicious new releases.

In `@src/clients/apiReportClient/apiReportClient.ts`:
- Around line 6-8: Ensure we fail fast if the environment var is missing: check
getEnvValue('VITE_APP_API_REPORT_URI') before creating axiosClient and throw a
clear error (or process.exit) when it's undefined/empty so axios.create({
baseURL: ... }) never receives an undefined baseURL; update the module that
defines axiosClient (the axiosClient constant in apiReportClient.ts) to read the
env into a local variable, validate it, and only then pass it into axios.create.

In `@src/index.tsx`:
- Around line 26-28: The tracePropagationTargets value produced by
getEnvValue('VITE_APP_SENTRY_TRACE_PROPAGATION_TARGETS') is split into [''] when
unset, which `@sentry/browser` treats as a wildcard; update the logic that builds
tracePropagationTargets so you split the env string, trim each entry, filter out
empty strings, and only include the tracePropagationTargets option when the
resulting array has at least one entry (otherwise omit the option so Sentry uses
its default allowlist). Reference the tracePropagationTargets assignment and the
getEnvValue call to locate and replace the current .split(',') handling.

In `@src/utils/getLinkedEventsInternalId.ts`:
- Around line 4-8: The helper getLinkedEventsInternalId builds a URL using
getEnvValue('VITE_APP_LINKEDEVENTS_API_URI') without guarding for a
missing/empty value, which can produce invalid IDs; update
getLinkedEventsInternalId to validate the env value returned by
getEnvValue('VITE_APP_LINKEDEVENTS_API_URI') (throw or return a clear
error/empty result) before interpolating, and construct the path using a
normalized base (ensure no duplicate slashes) so functions referencing
getLinkedEventsInternalId get only well-formed URLs.

---

Outside diff comments:
In `@src/test/apollo-mocks/headerMenuMock.ts`:
- Around line 15-527: The footer mock still uses padded base64 IDs (e.g.,
menu.id 'dGVybTo1Nw==' and menuItems.nodes[].id 'cG9zdDo0MjY=', 'cG9zdDo0Mjc=')
while headerMenuMock uses unpadded IDs; update
src/test/apollo-mocks/footerMenuMock.ts to use the unpadded forms (remove
trailing '=' padding) for the same fields so they match the MenuQueryResponse
shape used elsewhere (update the values for menu.id and each
menuItems.nodes[].id), or add a short comment in the file documenting that
padded IDs are intentionally kept if that was the deliberate choice.

---

Nitpick comments:
In @.env.runtime.template:
- Around line 23-24: Add a short comment above VITE_APP_APPLICATION_NAME and
VITE_APP_VERSION explaining these values are auto-populated/overridden by the
generator script from package.json (so values in this template are ignored) or
remove these lines entirely; update the .env.runtime.template to either include
that explanatory comment referencing VITE_APP_APPLICATION_NAME and
VITE_APP_VERSION or delete those two entries to avoid confusion.

In @.env.test:
- Around line 5-6: Remove the non-expanding placeholder assignments for
VITE_APP_APPLICATION_NAME and VITE_APP_VERSION in .env.test (the lines using
$npm_package_name and $npm_package_version); either delete these two lines or
replace them with concrete values (e.g., the actual app name and version) so
dotenv isn't given unusable placeholders—note the generator already pulls these
from package.json, so keep .env.test free of $npm_package_* placeholders and
rely on the generator or explicit literal values instead.

In @.github/workflows/ci.yml:
- Around line 11-16: The workflow currently inherits default permissions for the
reusable job "common" (uses:
City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@main); add an
explicit permissions block under that job to restrict token scopes to the
minimum required by the reusable workflow (e.g., explicitly set permissions:
contents: read, packages: read, pull-requests: write or whatever the reusable
workflow needs) instead of relying on defaults—review the reusable workflow's
steps to determine exact scopes and add only those permission keys.

In `@scripts/update-runtime-env.ts`:
- Around line 28-31: The path for configurationFile is built with string
concatenation; update the construction to use path.join for cross-platform
correctness. In the configurationFile assignment (the variable named
configurationFile that currently uses __dirname and configFile), call path.join
with components (e.g., __dirname, '..', 'public', configFile) instead of
concatenating '../public/' + configFile so separators are handled correctly
across OSes.

In `@src/utils/__tests__/envUtils.test.ts`:
- Around line 5-34: Add a test that verifies getEnvValue falls back to
process.env: set (globalThis.window as Window & { _env_?: Record<string,
unknown> })._env_ = undefined, assign process.env.TEST_ENV_UTILS_VALUE = '456'
(or another distinct value), then assert
expect(getEnvValue('TEST_ENV_UTILS_VALUE')).toBe('456'); name the test to
reflect the "process.env fallback" and rely on the existing afterEach cleanup
that deletes TEST_ENV_UTILS_VALUE and restores process.env/runtime env.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9c10b52b-95d3-4760-8eca-af9b0edac3cd

📥 Commits

Reviewing files that changed from the base of the PR and between 8ddcf15 and 0a9c4ec.

⛔ Files ignored due to path filters (5)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
  • src/domain/app/__tests__/__snapshots__/apolloRHHC.test.tsx.snap is excluded by !**/*.snap
  • src/domain/app/header/__tests__/__snapshots__/Header.test.tsx.snap is excluded by !**/*.snap
  • src/domain/app/layout/__tests__/__snapshots__/PageLayout.test.tsx.snap is excluded by !**/*.snap
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (32)
  • .dockerignore
  • .env.development.local.example
  • .env.example
  • .env.runtime.template
  • .env.test
  • .github/workflows/ci.yml
  • .gitignore
  • .husky/pre-commit
  • .nvmrc
  • Dockerfile
  • README.md
  • compose.yml
  • index.html
  • package.json
  • pipelines/kultus-admin-ui-review.yml
  • playwright.config.ts
  • pnpm-workspace.yaml
  • scripts/tsconfig.json
  • scripts/update-runtime-env.ts
  • src/clients/apiReportClient/apiReportClient.ts
  • src/domain/app/AppConfig.ts
  • src/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsx
  • src/domain/venue/utils.ts
  • src/headless-cms/components/__tests__/CmsPage.test.tsx
  • src/index.tsx
  • src/playwright/README.md
  • src/test/apollo-mocks/headerMenuMock.ts
  • src/test/vitest-setup.ts
  • src/utils/__tests__/envUtils.test.ts
  • src/utils/envUtils.ts
  • src/utils/getLinkedEventsInternalId.ts
  • src/vite-env.d.ts

Comment thread .dockerignore Outdated
Comment thread .env.test Outdated
Comment thread .github/workflows/ci.yml
Comment thread package.json Outdated
Comment thread pipelines/kultus-admin-ui-review.yml Outdated
Comment thread pnpm-workspace.yaml Outdated
Comment thread src/clients/apiReportClient/apiReportClient.ts
Comment thread src/index.tsx
Comment thread src/utils/getLinkedEventsInternalId.ts
@azure-pipelines

Copy link
Copy Markdown

KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀

@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 0a9c4ec to d1fafde Compare June 4, 2026 11:56
Comment thread Dockerfile
Comment thread src/clients/apiReportClient/apiReportClient.ts Outdated
@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from d1fafde to 48a8796 Compare June 4, 2026 12:06

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

10-16: ⚡ Quick win

Declare explicit least-privilege workflow permissions.

This workflow has no permissions: block, so token scope depends on repo/org defaults. Set explicit minimal scopes to harden CI behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 10 - 16, Add an explicit
least-privilege permissions block to the workflow job (the "common" job that
uses City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@main) so the
workflow token isn't granted repo/org defaults; insert a top-level permissions:
mapping that declares only the scopes required by your CI (for example at
minimum permissions: contents: read and actions: read, and add checks: write or
id-token: write only if the shared workflow requires them) to harden the
workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Dockerfile`:
- Line 19: The Dockerfile COPY currently uses a wildcard `.env*` which can
accidentally include local secret files; update the COPY command that includes
index.html, vite.config.ts, eslint.config.mjs, .prettierrc.json to explicitly
list only allowed env template files (e.g., .env.example or .env.production)
instead of `.env*`, ensuring you preserve the --chown=default:root flag and only
add the specific filenames you intend to bake into the image so secrets like
.env.local are not copied into image layers.

In `@README.md`:
- Line 310: The ordered list in README.md has a numbering gap: the step "Run the
Node application in development mode with `pnpm dev`. At this point you should
be also running the local APIs if you have selected to use them." is labeled
`5.` but should follow the previous top-level `3.` — change that list item to
`4.` (or renumber the sequence so top-level steps are sequential) and verify
nested items/indentation are correct so Markdown renders the steps in proper
order.

In `@scripts/update-runtime-env.ts`:
- Around line 17-23: The code is currently serializing the entire env map (env)
into the public browser config, exposing non-public keys; update the logic that
writes/serializes env so it filters env to only include public keys (e.g., keys
that start with "VITE_APP_" or your chosen public prefix) before creating the JS
output. Locate the dotenv.config usage and the code that writes the browser
config from env (references: env, USE_TEST_ENV) and replace the direct
serialization with a new object built by iterating env and copying only keys
matching /^VITE_APP_/ (or configured public prefix), then serialize that
filtered object into the generated file. Ensure override behavior is preserved
and no other env values are written to the client bundle.

---

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 10-16: Add an explicit least-privilege permissions block to the
workflow job (the "common" job that uses
City-of-Helsinki/.github/.github/workflows/ci-pnpm-node.yml@main) so the
workflow token isn't granted repo/org defaults; insert a top-level permissions:
mapping that declares only the scopes required by your CI (for example at
minimum permissions: contents: read and actions: read, and add checks: write or
id-token: write only if the shared workflow requires them) to harden the
workflow.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 51ddee04-8951-4056-90d7-87cc2866e11d

📥 Commits

Reviewing files that changed from the base of the PR and between 0a9c4ec and d1fafde.

⛔ Files ignored due to path filters (5)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
  • src/domain/app/__tests__/__snapshots__/apolloRHHC.test.tsx.snap is excluded by !**/*.snap
  • src/domain/app/header/__tests__/__snapshots__/Header.test.tsx.snap is excluded by !**/*.snap
  • src/domain/app/layout/__tests__/__snapshots__/PageLayout.test.tsx.snap is excluded by !**/*.snap
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (33)
  • .dockerignore
  • .env.development.local.example
  • .env.example
  • .env.runtime.template
  • .env.test
  • .github/workflows/ci.yml
  • .gitignore
  • .husky/pre-commit
  • .nvmrc
  • Dockerfile
  • README.md
  • compose.yml
  • index.html
  • package.json
  • pipelines/kultus-admin-ui-review.yml
  • playwright.config.ts
  • pnpm-workspace.yaml
  • scripts/tsconfig.json
  • scripts/update-runtime-env.ts
  • src/clients/apiReportClient/apiReportClient.ts
  • src/domain/app/AppConfig.ts
  • src/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsx
  • src/domain/venue/utils.ts
  • src/headless-cms/components/__tests__/CmsPage.test.tsx
  • src/index.tsx
  • src/playwright/README.md
  • src/test/apollo-mocks/footerMenuMock.ts
  • src/test/apollo-mocks/headerMenuMock.ts
  • src/test/vitest-setup.ts
  • src/utils/__tests__/envUtils.test.ts
  • src/utils/envUtils.ts
  • src/utils/getLinkedEventsInternalId.ts
  • src/vite-env.d.ts
✅ Files skipped from review due to trivial changes (10)
  • .nvmrc
  • .env.development.local.example
  • scripts/tsconfig.json
  • .env.example
  • src/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsx
  • src/test/apollo-mocks/footerMenuMock.ts
  • index.html
  • src/playwright/README.md
  • src/domain/venue/utils.ts
  • .gitignore
🚧 Files skipped from review as they are similar to previous changes (15)
  • src/test/vitest-setup.ts
  • src/headless-cms/components/tests/CmsPage.test.tsx
  • pipelines/kultus-admin-ui-review.yml
  • src/index.tsx
  • .dockerignore
  • src/utils/getLinkedEventsInternalId.ts
  • .husky/pre-commit
  • src/utils/tests/envUtils.test.ts
  • playwright.config.ts
  • src/clients/apiReportClient/apiReportClient.ts
  • compose.yml
  • src/utils/envUtils.ts
  • src/test/apollo-mocks/headerMenuMock.ts
  • src/domain/app/AppConfig.ts
  • package.json

Comment thread Dockerfile
Comment thread README.md
Comment thread scripts/update-runtime-env.ts
@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 48a8796 to 0eb0883 Compare June 4, 2026 12:13

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
Dockerfile (1)

19-19: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace .env* with an explicit allowlist.

This still copies every .env* file from the build context into the layer. Keep the template/example files explicit so local secrets cannot be baked into the image by accident.

🧩 Suggested fix
-COPY --chown=default:root index.html vite.config.ts eslint.config.mjs .prettierrc.json .env* ./
+COPY --chown=default:root index.html vite.config.ts eslint.config.mjs .prettierrc.json ./
+COPY --chown=default:root .env.runtime.template .env.example .env.development.local.example .env.test ./
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Dockerfile` at line 19, The Dockerfile COPY line currently uses a glob
`.env*` which risks including local secret env files; update the COPY invocation
that contains "COPY --chown=default:root index.html vite.config.ts
eslint.config.mjs .prettierrc.json .env* ./" to explicitly list only safe
template/example env files (for example `.env.example` and/or `.env.template`)
instead of the `.env*` glob so no accidental local `.env` or `.env.local` files
are baked into the image.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Dockerfile`:
- Around line 16-19: The Dockerfile runs the command pnpm update-runtime-env
before the .env.runtime.template is added to the image, causing builds to fail
or produce wrong runtime config; fix by ensuring .env.runtime.template (or other
env templates) are copied into the image before invoking pnpm
update-runtime-env—either move the pnpm update-runtime-env step to after the
COPY that includes .env* or add an earlier COPY that places
.env.runtime.template into the image prior to running pnpm update-runtime-env so
the update-runtime-env script can read the template.

In `@src/headless-cms/components/__tests__/CmsPage.test.tsx`:
- Around line 124-131: Update the test comment to correctly describe why we pass
deterministic overrides to fakePost: the id/postId values are used only as React
keys in CmsSidebarContent (not as DOM id attributes for LayoutArticles); the
sidebar HTML id is generated from LayoutLinkList.anchor in
CmsSidebarContentLayoutLinkList.tsx, which is what the test queries—so change or
remove the current comment to state that deterministic values keep React keys
stable rather than claiming jsdom selector safety.

---

Duplicate comments:
In `@Dockerfile`:
- Line 19: The Dockerfile COPY line currently uses a glob `.env*` which risks
including local secret env files; update the COPY invocation that contains "COPY
--chown=default:root index.html vite.config.ts eslint.config.mjs
.prettierrc.json .env* ./" to explicitly list only safe template/example env
files (for example `.env.example` and/or `.env.template`) instead of the `.env*`
glob so no accidental local `.env` or `.env.local` files are baked into the
image.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: beaf6739-4cdb-4e63-af42-cfdf0e783607

📥 Commits

Reviewing files that changed from the base of the PR and between d1fafde and 48a8796.

⛔ Files ignored due to path filters (5)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
  • src/domain/app/__tests__/__snapshots__/apolloRHHC.test.tsx.snap is excluded by !**/*.snap
  • src/domain/app/header/__tests__/__snapshots__/Header.test.tsx.snap is excluded by !**/*.snap
  • src/domain/app/layout/__tests__/__snapshots__/PageLayout.test.tsx.snap is excluded by !**/*.snap
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (33)
  • .dockerignore
  • .env.development.local.example
  • .env.example
  • .env.runtime.template
  • .env.test
  • .github/workflows/ci.yml
  • .gitignore
  • .husky/pre-commit
  • .nvmrc
  • Dockerfile
  • README.md
  • compose.yml
  • index.html
  • package.json
  • pipelines/kultus-admin-ui-review.yml
  • playwright.config.ts
  • pnpm-workspace.yaml
  • scripts/tsconfig.json
  • scripts/update-runtime-env.ts
  • src/clients/apiReportClient/apiReportClient.ts
  • src/domain/app/AppConfig.ts
  • src/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsx
  • src/domain/venue/utils.ts
  • src/headless-cms/components/__tests__/CmsPage.test.tsx
  • src/index.tsx
  • src/playwright/README.md
  • src/test/apollo-mocks/footerMenuMock.ts
  • src/test/apollo-mocks/headerMenuMock.ts
  • src/test/vitest-setup.ts
  • src/utils/__tests__/envUtils.test.ts
  • src/utils/envUtils.ts
  • src/utils/getLinkedEventsInternalId.ts
  • src/vite-env.d.ts
✅ Files skipped from review due to trivial changes (12)
  • .nvmrc
  • src/index.tsx
  • src/domain/venue/utils.ts
  • compose.yml
  • .env.development.local.example
  • src/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsx
  • src/vite-env.d.ts
  • src/playwright/README.md
  • .env.example
  • index.html
  • .gitignore
  • .husky/pre-commit
🚧 Files skipped from review as they are similar to previous changes (13)
  • src/test/vitest-setup.ts
  • scripts/tsconfig.json
  • playwright.config.ts
  • src/utils/getLinkedEventsInternalId.ts
  • .dockerignore
  • src/utils/tests/envUtils.test.ts
  • pnpm-workspace.yaml
  • src/test/apollo-mocks/footerMenuMock.ts
  • scripts/update-runtime-env.ts
  • package.json
  • src/test/apollo-mocks/headerMenuMock.ts
  • src/domain/app/AppConfig.ts
  • src/utils/envUtils.ts

Comment thread Dockerfile
Comment thread src/headless-cms/components/__tests__/CmsPage.test.tsx
Comment thread src/utils/envUtils.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Around line 381-383: The README’s doctoc example command ("pnpm doctoc .")
does not match the pre-commit hook which runs "pnpm doctoc . -u"; update the
README entry to use the exact command used by the hook ("pnpm doctoc . -u") so
docs match behavior, or alternatively change the hook to remove the "-u"
flag—make the change where the README lists the doctoc command and ensure the
string "pnpm doctoc . -u" is used if choosing to align docs to the hook.
- Line 386: Update the README line that points to the lint-staged config:
replace the incorrect reference to "./lint-staged.config.js" with an instruction
that the lint-staged configuration lives in package.json under the "lint-staged"
field (e.g., mention "see package.json -> lint-staged"). Ensure the command
description remains the same and that the README clearly directs readers to the
package.json lint-staged entry.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 12436551-56f7-48d6-a97a-0dd0280dee78

📥 Commits

Reviewing files that changed from the base of the PR and between 48a8796 and 0eb0883.

⛔ Files ignored due to path filters (5)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
  • src/domain/app/__tests__/__snapshots__/apolloRHHC.test.tsx.snap is excluded by !**/*.snap
  • src/domain/app/header/__tests__/__snapshots__/Header.test.tsx.snap is excluded by !**/*.snap
  • src/domain/app/layout/__tests__/__snapshots__/PageLayout.test.tsx.snap is excluded by !**/*.snap
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (33)
  • .dockerignore
  • .env.development.local.example
  • .env.example
  • .env.runtime.template
  • .env.test
  • .github/workflows/ci.yml
  • .gitignore
  • .husky/pre-commit
  • .nvmrc
  • Dockerfile
  • README.md
  • compose.yml
  • index.html
  • package.json
  • pipelines/kultus-admin-ui-review.yml
  • playwright.config.ts
  • pnpm-workspace.yaml
  • scripts/tsconfig.json
  • scripts/update-runtime-env.ts
  • src/clients/apiReportClient/apiReportClient.ts
  • src/domain/app/AppConfig.ts
  • src/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsx
  • src/domain/venue/utils.ts
  • src/headless-cms/components/__tests__/CmsPage.test.tsx
  • src/index.tsx
  • src/playwright/README.md
  • src/test/apollo-mocks/footerMenuMock.ts
  • src/test/apollo-mocks/headerMenuMock.ts
  • src/test/vitest-setup.ts
  • src/utils/__tests__/envUtils.test.ts
  • src/utils/envUtils.ts
  • src/utils/getLinkedEventsInternalId.ts
  • src/vite-env.d.ts
✅ Files skipped from review due to trivial changes (9)
  • .nvmrc
  • src/headless-cms/components/tests/CmsPage.test.tsx
  • src/domain/venue/utils.ts
  • .env.example
  • .env.development.local.example
  • src/playwright/README.md
  • src/utils/envUtils.ts
  • src/test/apollo-mocks/footerMenuMock.ts
  • .gitignore
🚧 Files skipped from review as they are similar to previous changes (17)
  • index.html
  • playwright.config.ts
  • pipelines/kultus-admin-ui-review.yml
  • src/vite-env.d.ts
  • scripts/tsconfig.json
  • src/index.tsx
  • src/clients/apiReportClient/apiReportClient.ts
  • .dockerignore
  • .husky/pre-commit
  • src/utils/getLinkedEventsInternalId.ts
  • pnpm-workspace.yaml
  • scripts/update-runtime-env.ts
  • src/utils/tests/envUtils.test.ts
  • compose.yml
  • package.json
  • src/test/apollo-mocks/headerMenuMock.ts
  • src/domain/app/AppConfig.ts

Comment thread README.md
Comment thread README.md
@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 0eb0883 to 081095b Compare June 4, 2026 12:21
Comment thread Dockerfile

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 11-16: Add an explicit GitHub Actions permissions block
immediately under the reusable workflow invocation (the common: uses:
City-of-Helsinki/... entry) to avoid default token scopes; update the job that
contains the uses: line (the "common" job) to include a minimal permissions set
that only grants what CI needs (for example, at minimum contents: read and
actions: read, and add checks: write or id-token: write only if the workflow
actually requires them)—ensure you place the new permissions: block at the same
level as uses:, secrets:, and with: and remove reliance on default scopes.

In `@index.html`:
- Around line 51-52: Update the development command in the HTML template
comment: replace the incorrect "pnpm start" string with the project's actual dev
command "pnpm dev" in the comment block that currently reads "To begin the
development, run `pnpm start`." so the docs match the real script used by the
project.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2134d53c-e87f-4140-9937-825496e446b3

📥 Commits

Reviewing files that changed from the base of the PR and between 0eb0883 and 081095b.

⛔ Files ignored due to path filters (5)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
  • src/domain/app/__tests__/__snapshots__/apolloRHHC.test.tsx.snap is excluded by !**/*.snap
  • src/domain/app/header/__tests__/__snapshots__/Header.test.tsx.snap is excluded by !**/*.snap
  • src/domain/app/layout/__tests__/__snapshots__/PageLayout.test.tsx.snap is excluded by !**/*.snap
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (33)
  • .dockerignore
  • .env.development.local.example
  • .env.example
  • .env.runtime.template
  • .env.test
  • .github/workflows/ci.yml
  • .gitignore
  • .husky/pre-commit
  • .nvmrc
  • Dockerfile
  • README.md
  • compose.yml
  • index.html
  • package.json
  • pipelines/kultus-admin-ui-review.yml
  • playwright.config.ts
  • pnpm-workspace.yaml
  • scripts/tsconfig.json
  • scripts/update-runtime-env.ts
  • src/clients/apiReportClient/apiReportClient.ts
  • src/domain/app/AppConfig.ts
  • src/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsx
  • src/domain/venue/utils.ts
  • src/headless-cms/components/__tests__/CmsPage.test.tsx
  • src/index.tsx
  • src/playwright/README.md
  • src/test/apollo-mocks/footerMenuMock.ts
  • src/test/apollo-mocks/headerMenuMock.ts
  • src/test/vitest-setup.ts
  • src/utils/__tests__/envUtils.test.ts
  • src/utils/envUtils.ts
  • src/utils/getLinkedEventsInternalId.ts
  • src/vite-env.d.ts
✅ Files skipped from review due to trivial changes (9)
  • .nvmrc
  • .husky/pre-commit
  • src/domain/venue/utils.ts
  • src/playwright/README.md
  • src/headless-cms/components/tests/CmsPage.test.tsx
  • src/vite-env.d.ts
  • .gitignore
  • .env.development.local.example
  • src/domain/occurrence/enrolmentTable/additionalInfo/AdditionalInfo.tsx
🚧 Files skipped from review as they are similar to previous changes (17)
  • src/test/vitest-setup.ts
  • .env.example
  • src/utils/getLinkedEventsInternalId.ts
  • compose.yml
  • scripts/tsconfig.json
  • src/test/apollo-mocks/footerMenuMock.ts
  • pipelines/kultus-admin-ui-review.yml
  • src/index.tsx
  • src/utils/envUtils.ts
  • playwright.config.ts
  • src/utils/tests/envUtils.test.ts
  • scripts/update-runtime-env.ts
  • pnpm-workspace.yaml
  • package.json
  • src/test/apollo-mocks/headerMenuMock.ts
  • src/domain/app/AppConfig.ts
  • .dockerignore

Comment thread .github/workflows/ci.yml
Comment thread index.html
@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 081095b to 6ddcdf9 Compare June 4, 2026 12:30
Comment thread scripts/update-runtime-env.ts
@azure-pipelines

Copy link
Copy Markdown

KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀

@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 6ddcdf9 to 15fc981 Compare June 8, 2026 11:11
Comment thread src/utils/envUtils.ts
Comment thread scripts/update-runtime-env.ts
@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 15fc981 to ca4b3de Compare June 8, 2026 11:19
@azure-pipelines

Copy link
Copy Markdown

KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀

@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from ca4b3de to 2c27577 Compare June 8, 2026 11:43
Comment thread src/utils/envUtils.ts Outdated
@tjgofore tjgofore force-pushed the feat/PT-2045-replace-yarn-with-pnpm branch from 2c27577 to 45411fb Compare June 8, 2026 11:48
@sonarqubecloud

sonarqubecloud Bot commented Jun 8, 2026

Copy link
Copy Markdown

@azure-pipelines

Copy link
Copy Markdown

KULTUS-ADMIN-UI branch is deployed to platta: https://kultus-admin-ui-pr471.dev.hel.ninja 🚀🚀🚀

@tjgofore tjgofore merged commit 49929df into main Jun 8, 2026
26 checks passed
@tjgofore tjgofore deleted the feat/PT-2045-replace-yarn-with-pnpm branch June 8, 2026 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants